[Feature] : API ENDPOINTS PR 1: Foundation and scaffolding#1130
Conversation
cfsmp3
left a comment
There was a problem hiding this comment.
Note the two high complexity errors (validation.py and status.py), also see Claude review below.
Reading the rview, clearly H1 must be addressed before we can merge or things will break.
The rest could be addressed (possibly are) later on in the stack.
But let's make sure we can merge and test each PR in order but not at the same time, i.e. merging this diff shouldn't break the system, even if by itself it doesn't do anything useful (since it's scaffolding).
Claude review follows:
HIGH (blocker):
- H1 — ApiToken model added with NO migration. The api_token table won't exist on real MySQL. Tests pass only because tests/base.py does create_all from models → masks the gap. PR2's auth endpoints break at runtime. #1117 had this migration (d4f8e2a1b3c7); dropped in the split. Must add it, chained off master head c8f3a2b1d4e5.
MEDIUM (carryover, code unchanged):
- ~770 lines of security middleware merge untested — and with no routes yet, the before_request hooks never even fire in this PR. Deferred to PR2/3. Risk noted.
- Rate limiter unbounded memory (no hard cap, eviction only every 100 req).
- Auth timing oracle (no-candidate path skips argon2 verify → leaks whether a prefix exists).
- _get_client_ip comment wrong (ProxyFix means remote_addr is from XFF).
LOW/NIT: run-level missing→fail path untested; stale is_dummy_row "DEPLOYMENT PREREQUISITE" docstring; N+1 footgun in the batch_get_run_data wrappers; pytest conftest.py inert under nose2; generic 429 handler hardcodes wrong limits.
9950853 to
beb4fe9
Compare
|
New / still open:
|
b9a645a to
d1e2392
Compare
d1e2392 to
c69c72d
Compare
|



[Feature]
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Overview
First PR of a 7-part stack that adds a token-authenticated JSON REST API (
/api/v1) to the sample platform, giving CI consumers and future tooling programmatic access to runs, samples, results, and logs — everything that today requires scraping the HTML views.Stack: #1130 → #1131 → #1132 → #1133 → #1134 → #1135 → #1141. Merge bottom-up. Each PR contains the ones below it, so the diff shrinks automatically as parents merge.
This PR deliberately contains no endpoints. It lays down the infrastructure the six route PRs build on, so their reviews can focus on endpoint logic.
What's in it
Blueprint + middleware (
mod_api/)mod_apiblueprint registered at/api/v1. Anything under that prefix returns structured JSON errors ({code, message, details}) — including routing-level 404/405s, which are converted app-wide via anafter_app_requesthook so no HTML error page ever leaks into an API response.Authorization: Bearer spci_...(scheme matched case-insensitively per RFC 7235). Tokens are 256-bit random secrets; the server stores only a SHA-256 hash plus an indexed 16-char prefix for lookup. Verification fetches candidates by prefix and compares hashes withhmac.compare_digest(constant-time). Scope checks (@require_scope) and role checks (@require_roles) are separate decorators, so each endpoint's requirements are visible at its definition.X-RateLimit-*response headers. In-memory by design — single-process deployment; the store interface is small enough to swap for Redis if that ever changes.after_request) is documented inmod_api/__init__.py.Model + migration
ApiTokenmodel: name (unique per user, kept after revocation for audit), hash, prefix, JSON scopes, expiry, revocation timestamps.d4f8e2a1b3c7: newapi_tokentable + nullableuser.github_logincolumn. Purely additive — no existing table is modified, so it's safe for the deployment pipeline to auto-apply, and rolling back the code without rolling back the schema is harmless.Status derivation service (
mod_api/services/status.py)The single source of truth mapping raw
TestProgress/TestResult/TestResultFilerows to normalized statuses (runs:queued/running/pass/fail/canceled/error/incomplete; samples:pass/fail/skipped/missing_output/running/not_started). Route handlers are forbidden from inlining their own derivation — this is what keeps/summary,/samples, and/errorsfrom ever disagreeing. Documented data-model traps it encodes:TestResultFile.got = NULLmeans match, not missing; the(-1, 'error')dummy row means the test produced no output;test.failedonly reflects cancellation. A run marked completed that produced zero result rows reportserror, neverpass.Test scaffolding
tests/api/base.pyprovidesApiTestCase, which stubs the deliberately-slowsha512_cryptpassword hashing for API tests (they create users/tokens constantly). This keeps the API suite in the seconds range instead of minutes; any test needing real hashing can stop the patchers locally.Testing
42 tests: token model (generation, hashing, prefix extraction, expiry), status derivation (including the completed-without-results →
errorcase), validation helpers, and utils. Full lint battery (isort, pycodestyle, pydocstyle, dodgy, mypy) is green.